riscv64: Implement SIMD saturating arithmetic and min/max #6430
Conversation
github-actions
bot
added
the
cranelift
| (vec_alu_rrr (VecAluOpRRR.VminuVV) vs2 vs1 mask vstate)) | ||
|
|
||
| ;; Helper for emitting the `vminu.vx` instruction. | ||
| (decl rv_vminu_vx (Reg Reg VecOpMasking VState) Reg) |
There was a problem hiding this comment.
Stray thought, but the x64 backend has different types for Gpr and Xmm which represent a type-level distinction for different classes of registers. I've found that it's actually worked quite well for x64 and it's saved me a few times from mistakes. I realized I should be carefully reading the rules that use *_vx instructions to ensure the variable bound by splat comes second instead of first by accident (since I think that would compile past ISLE but wouldn't get past debug asserts perhaps in the backend).
Having this sort of distinction though is a pretty large refactoring so definitely not necessary on this PR, but perhaps something to consider if you also find yourself trying to carefully ensure each register goes to the right spot.
There was a problem hiding this comment.
Yeah, that sounds like a good idea! And I've definitely had that happen to me. Not with .vx, but passing an F register into a .vx opcode. We do hit the regclass asserts in the backend, but it would be better to have it as a type system feature.
It should be fairly easy to start at least with the vector rules and helpers. I'm going to give that a try.
👋 Hey,
This PR Implements the saturating arithmetic (
{u,s}{add,sub}_sat) and min/max ({u,s}{min,max}) instructions.Only the saturating arithmetic functions are tested via WASM tests. The min/max instruction testsuites have other instructions mixed in that will be implemented in a future PR so we can't enable them yet. These are currently only tested via cranelift tests.
I've also added i128 tests for min/max since I accidentally disabled these and found we didn't have anything catching that case.